Nat rule migration improvements #161
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies NAT rule migration by removing asynchronous job handling and improving log timestamp precision. The changes focus on streamlining the load balancer update process and enhancing logging capabilities.
- Replaced asynchronous Set-AzLoadBalancer job with synchronous call
- Enhanced log timestamp format to include milliseconds
- Updated module version and release notes
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| NatRulesMigration.psm1 | Simplified load balancer update from async job to synchronous call |
| Log.psm1 | Enhanced timestamp format to include milliseconds for better precision |
| AzureBasicLoadBalancerUpgrade.psd1 | Updated module version and release notes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
...pgrade/module/AzureBasicLoadBalancerUpgrade/modules/NatRulesMigration/NatRulesMigration.psm1
Outdated
Show resolved
Hide resolved
| If ($UpdateLBNATRulesJob.Error -or $UpdateLBNATRulesJob.State -eq 'Failed') { | ||
| log -Severity Error -Message "Saving Standard Load Balancer $($StdLoadBalancer.Name) failed with the following errors: $($UpdateLBNATRulesJob.error; $UpdateLBNATRulesJob | Receive-Job). Migration will continue--to recover, manually add the NAT rules to the load balancer, then correct NAT Rule NIC membership after the script completes." | ||
| } | ||
| Set-AzLoadBalancer -LoadBalancer $StdLoadBalancer > $null |
There was a problem hiding this comment.
Removing the asynchronous job handling eliminates progress feedback and detailed error information that was previously available. The original implementation provided status updates during long-running operations and captured specific job errors, which could be valuable for troubleshooting.
Suggested change
| Set-AzLoadBalancer -LoadBalancer $StdLoadBalancer > $null | |
| $job = Set-AzLoadBalancer -LoadBalancer $StdLoadBalancer -AsJob | |
| log -Message "[NatRulesMigration] Set-AzLoadBalancer job started (Id: $($job.Id)). Monitoring progress..." | |
| while ($job.State -eq 'Running' -or $job.State -eq 'NotStarted') { | |
| log -Message "[NatRulesMigration] Set-AzLoadBalancer job status: $($job.State)..." | |
| Start-Sleep -Seconds 5 | |
| $job = Get-Job -Id $job.Id | |
| } | |
| if ($job.State -eq 'Completed') { | |
| log -Message "[NatRulesMigration] Set-AzLoadBalancer job completed successfully." | |
| # Optionally, retrieve job output if needed | |
| # $output = Receive-Job -Id $job.Id | |
| } | |
| elseif ($job.State -eq 'Failed') { | |
| $errorMsg = "[NatRulesMigration] Set-AzLoadBalancer job failed. Error: $((Receive-Job -Id $job.Id | Out-String))" | |
| log "Error" $errorMsg | |
| } | |
| else { | |
| $errorMsg = "[NatRulesMigration] Set-AzLoadBalancer job ended with unexpected state: $($job.State)." | |
| log "Error" $errorMsg | |
| } | |
| # Clean up job | |
| Remove-Job -Id $job.Id |
…rade/modules/NatRulesMigration/NatRulesMigration.psm1 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
mbrat2005
added a commit
that referenced
this pull request
Dec 19, 2025
* add milliseconds to logging * Remove use of AzJob for LB save to simplify flow * v2.5.40 * Update AzureBasicLoadBalancerUpgrade/module/AzureBasicLoadBalancerUpgrade/modules/NatRulesMigration/NatRulesMigration.psm1 Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.